Skip to content

Enable using cppyy with debug build of Python#56

Draft
Vipul-Cariappa wants to merge 1 commit intowlav:masterfrom
Vipul-Cariappa:dev/pydebug
Draft

Enable using cppyy with debug build of Python#56
Vipul-Cariappa wants to merge 1 commit intowlav:masterfrom
Vipul-Cariappa:dev/pydebug

Conversation

@Vipul-Cariappa
Copy link

The changes here collects residual exceptions thrown by Python, before calling into Python C API.

Reference: root-project/root#19239

The changes here collects residual exceptions thrown by Python,
before calling into Python C API.
@wlav
Copy link
Owner

wlav commented Jul 3, 2025

I really don't think straight-up sprinkling PyErr_Clear() b/c the "might" be an error is a good idea. Presumably, the error is there for a reason and even if it's only in debug builds, the error should be handled/collected close to its source.

Copy link
Author

@Vipul-Cariappa Vipul-Cariappa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the explanation:

}
}

PyErr_Clear(); // creation might have failed
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This happens when meta_getattro fails. In my investigation, it failed for some special methods. And if the user does something like:

In [3]: cppyy.cppdef("""
   ...: namespace ns {}
   ...: """)
Out[3]: True

In [4]: gbl.ns.something = 1

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, now that I think about this, in the above-mentioned cases, it should not be calling meta_getattro. I will need to cross-check this.

Comment on lines 1805 to +1806
Utility::AddToClass(pyclass, "__real_data", "data");
PyErr_Clear(); // AddToClass might have failed for data
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This failure is at

PyObject* pyfunc = PyObject_GetAttrString(pyclass, const_cast<char*>(func));

With pyclass and "data" as arguments.
I guess this is one of the errors that we should not clear/ignore. I will look into it.

Comment on lines +555 to +557
PyObject* argtypes = nullptr;
PyObject* ret = nullptr;
if ((argtypes = PyObject_GetAttrString(arg, "argtypes")) && (ret = PyObject_GetAttrString(arg, "restype"))) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a safer rewrite for the PyErr_Clear in line 582 below.
The first GetAttr fails, then in the second, Python's assert(!PyErr_Occurred()) fails.

Comment on lines 896 to +898
if (PySequence_Check(pyobject) && !PySequence_Size(pyobject))
return 0; // PyObject_GetBuffer() crashes on some platforms for some zero-sized seqeunces
PyErr_Clear();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This happens when PySequence_Size fails with an error something like "sequence does not have __len__", but PySequence_Check succeeds. (I will cross-check, once)

@Vipul-Cariappa Vipul-Cariappa marked this pull request as draft July 4, 2025 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants